Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(list): Do not fetch directory URIs in stat #3093

Merged
merged 16 commits into from
Sep 9, 2024
Merged

fix(list): Do not fetch directory URIs in stat #3093

merged 16 commits into from
Sep 9, 2024

Conversation

traeok
Copy link
Member

@traeok traeok commented Sep 5, 2024 •
edited
Loading

Proposed changes

There was an oversight when implementing the remote lookup functionality for Data Sets and USS. The stat implementations should never make an API call unless we are explicitly fetching the URI with the fetch query, or if the entry is a file (stat is called for files open in the editor).

Considering this causes a balloon in API calls when listing data sets and USS folders, it would be best to get this fix into the upcoming pre-release, or at the very least get the fix merged before v3 GA so that our users don't encounter this problem 😅

How to test:
NOTE: Please test on a dev LPAR as this may spam the mainframe with API calls
List a large data set in Zowe Explorer. In this PR it will load very quickly - in the last pre-release it will take quite some time to complete (or it won't finish loading at all)

Release Notes

Milestone: vNext

Changelog:

  • Fixed issue where listing data sets or USS files would cause a drastic increase in API calls, causing delays or a complete halt in Zowe Explorer.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs
    • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
    • Otherwise, please check with the rest of the squad about any needed documentation before merging.
  • These changes may need ported to the appropriate branches (list here):

@traeok traeok changed the base branch from main to next September 5, 2024 15:58
@pull-request-size pull-request-size bot added size/S and removed size/XS labels Sep 5, 2024
@codecov Codecov
Copy link

codecov bot commented Sep 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.77%. Comparing base (6bc3179) to head (3c5c3a5).
Report is 17 commits behind head on next.

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #3093      +/-   ##
==========================================
+ Coverage   92.73%   92.77%   +0.03%     
==========================================
  Files         113      113              
  Lines       11566    11608      +42     
  Branches     2553     2572      +19     
==========================================
+ Hits        10726    10769      +43     
+ Misses        838      837       -1     
  Partials        2        2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@traeok traeok marked this pull request as ready for review September 5, 2024 17:46
@traeok traeok changed the title fix(list): Do not fetch URIs when query parameter is not specified fix(list): Do not fetch directory URIs in stat Sep 5, 2024
@pull-request-size pull-request-size bot added size/M and removed size/S labels Sep 5, 2024
@traeok traeok force-pushed the fix/listing-items-api-calls branch 2 times, most recently from d36a62e to bf910c5 Compare September 6, 2024 00:47
@traeok traeok force-pushed the fix/listing-items-api-calls branch from bf910c5 to 66d9fd3 Compare September 6, 2024 01:08
@traeok traeok mentioned this pull request Sep 6, 2024
Merged
15 tasks
@traeok traeok marked this pull request as draft September 6, 2024 13:41
@traeok

This comment was marked as outdated.

Sign in to view
@traeok
Sync changes with remote
e914076 
Signed-off-by: Trae Yelovich <[email protected]>
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 6, 2024
@traeok traeok marked this pull request as ready for review September 6, 2024 14:34
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 6, 2024

📅 Suggested merge-by date: 9/20/2024

zFernand0
zFernand0 previously approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes make sense to me! 🙏🏽
Thanks for addressing the issue so quickly! 🥳
LGTM! 😋

packages/zowe-explorer/src/trees/uss/UssFSProvider.ts Outdated Show resolved Hide resolved
packages/zowe-explorer/src/trees/uss/UssFSProvider.ts Show resolved Hide resolved
packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts Show resolved Hide resolved
packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts Show resolved Hide resolved
@traeok traeok requested a review from zFernand0 September 6, 2024 15:35
zFernand0
zFernand0 approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 😋

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Sep 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

JillieBeanSim
JillieBeanSim approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@JillieBeanSim JillieBeanSim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great and solves the hang mentioned on call last week retrieving data set members. Thanks @traeok for the fix!

@JillieBeanSim JillieBeanSim merged commit c28d75b into next Sep 9, 2024
27 checks passed
@JillieBeanSim JillieBeanSim deleted the fix/listing-items-api-calls branch September 9, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anaxceron anaxceron anaxceron approved these changes

@JillieBeanSim JillieBeanSim JillieBeanSim approved these changes

@zFernand0 zFernand0 zFernand0 approved these changes

@awharn awharn Awaiting requested review from awharn

@rudyflores rudyflores Awaiting requested review from rudyflores

@SanthoshiBoyina1 SanthoshiBoyina1 Awaiting requested review from SanthoshiBoyina1

@likhithanimma1 likhithanimma1 Awaiting requested review from likhithanimma1

Assignees
No one assigned
Labels
size/L
Projects
Zowe Explorer for VS Code
Status: Closed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@traeok @zFernand0 @JillieBeanSim @anaxceron